Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

addition of details for TIMESYS in the case of calendar years time st… #36

Merged
merged 3 commits into from
Sep 1, 2023
Merged

addition of details for TIMESYS in the case of calendar years time st… #36

merged 3 commits into from
Sep 1, 2023

Conversation

Bonnarel
Copy link
Contributor

…amps

@Bonnarel
Copy link
Contributor Author

Related to issue #21

msdemlei
msdemlei suggested changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@msdemlei msdemlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the somewhat unclear note about time formats in coosys, I'm fine with the change. It's a bit of a fine point, as the difference between ET and TDB will probably rarely matter when people write fractional years, but better safe than sorry.

VOTable.tex Outdated
@@ -681,7 +681,8 @@ \subsection{\elem{COOSYS} Element}
equatorial or ecliptic systems (as e.g., \verb|"J2000"| as the default
for \verb|"eq_FK5"| or \verb|"B1950"| as the default for
\verb|"eq_FK4"|), and \attr{epoch} specifies the epoch of the positions
if necessary. Note that the \elem{COOSYS} may be deprecated in the
if necessary. Both equinox and epoch MUST expressed in the Julian or Besselian calendar.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly are you trying to allow or forbid here? Is it just about unadorned years? If so, don't confuses people with a MUST and just say: "COOSYS only supports time specificiations in Julian or Besselian years"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I change the PR.

@tomdonaldson tomdonaldson added this to the v1.5 milestone May 11, 2023
tomdonaldson
tomdonaldson approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@tomdonaldson tomdonaldson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Bonnarel! I've resolved conflicts with #33 and fixed the build problem with the new reference, so this is ready to go.

@tomdonaldson tomdonaldson merged commit 4b6a2ac into ivoa-std:master Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msdemlei msdemlei msdemlei requested changes

@tomdonaldson tomdonaldson tomdonaldson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
3 participants
@Bonnarel @msdemlei @tomdonaldson